Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

test(ngMock): fix Firefox crashes on Travis #16040

Closed
wants to merge 1 commit into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jun 7, 2017

This test keeps causing Firefox 47 (currently used on Travis) to crash and fail the build. The test passes locally (on Firefox 53). Lowering the loop count from 1000 to 100 seems to fix the issue.
(Note: The crach only affects the mocked implementation of $interval and does not happen locally.)

@gkalpak gkalpak force-pushed the travis-driven-debugging branch from 0900ad0 to 0ee62f8 Compare June 8, 2017 13:28
@gkalpak gkalpak changed the title TEST - Revert "fix(ngMock/$interval): add support for zero-delay intervals in tests" chore(package.json): upgrade selenium-webdriver to 3.4.0 Jun 8, 2017
@gkalpak gkalpak added this to the Backlog milestone Jun 8, 2017
@gkalpak gkalpak changed the title chore(package.json): upgrade selenium-webdriver to 3.4.0 WIP - chore(package.json): upgrade selenium-webdriver to 3.4.0 Jun 8, 2017
@gkalpak gkalpak changed the title WIP - chore(package.json): upgrade selenium-webdriver to 3.4.0 WIP - chore(travis): debug Travis failures Jun 10, 2017
@gkalpak gkalpak force-pushed the travis-driven-debugging branch 2 times, most recently from 804b9b9 to 3c66f28 Compare June 11, 2017 08:05
@gkalpak gkalpak closed this Jun 11, 2017
@gkalpak gkalpak reopened this Jun 11, 2017
This test keeps causing Firefox 47 (currently used on Travis) to crash and fail
the build. The test passes locally (on Firefox 53). Lowering the loop count from
1000 to 100 seems to fix the issue.
(Note: The crach only affects the mocked implementation of `$interval` and does
not happen locally.)
@gkalpak gkalpak force-pushed the travis-driven-debugging branch from 20e3cec to 4a06791 Compare June 12, 2017 08:07
@gkalpak gkalpak changed the title WIP - chore(travis): debug Travis failures test(ngMock): fix Firefox crashes on Travis Jun 12, 2017
@Narretz
Copy link
Contributor

Narretz commented Jun 12, 2017

If I remember correctly, browsers can't actually guarantee an interval of 1ms. Maybe we should instead set it to 10ms to relieve the pressure? Or do you think the condition in the test with 1000 loops is unrealistic and doesn't need to be tested?

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Jun 12, 2017

But this is a mock $interval, right? We should have any reliance upon the real browser's ability to handler short intervals.

@Narretz
Copy link
Contributor

Narretz commented Jun 12, 2017

You are right, I read over the fact that this is only the mock implementation. But it looks like the "fake" interval of 1ms also causes the browsers in question problems - which could maybe be handled by using a slightly higher interval, which would also be more in line with what browsers do with real intervals.

@gkalpak
Copy link
Member Author

gkalpak commented Jun 12, 2017

I haven't tried it but I suspect it is something about the loop count that throws Firefox off. Even if we increased the faked interval to 10ms, using flush(10000) would result in the same number of iterations (and probably the same crashing). Given that this only affects the mock implementation and that I can't reproduce locally with latest Firefox, I am fine going either way. Note though, that this is not specific to the 0-delay interval (e.g. you can cause Firefox 47 to crash on Travis by explicitly using $interval(1) in a test.)

@Narretz Narretz modified the milestones: 1.6.5, Backlog Jun 12, 2017
@Narretz
Copy link
Contributor

Narretz commented Jun 12, 2017

Ok, in that case I'm fine with changing just the test

@petebacondarwin
Copy link
Contributor

OK.

@gkalpak gkalpak closed this in 53fb909 Jun 12, 2017
@gkalpak gkalpak deleted the travis-driven-debugging branch June 12, 2017 11:51
gkalpak added a commit that referenced this pull request Jun 12, 2017
This test keeps causing Firefox 47 (currently used on Travis) to crash and fail
the build. The test passes locally (on Firefox 53). Lowering the loop count from
1000 to 100 seems to fix the issue.
(Note: The crach only affects the mocked implementation of `$interval` and does
not happen locally.)

Closes #16040
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants